fix: require explicit refresh for bundle manifest changes - #4477
fix: require explicit refresh for bundle manifest changes#4477rksharma-owg wants to merge 4 commits into
Conversation
|
Thanks — this correctly fixes the primary #4434 failure (a version-changing normal install advancing the record past stale components), with a clean guard and regression test, and the disclosure is appreciated. One thing before merge: #4434 reported a second case that this doesn't cover — components installed individually before the bundle, where every primitive is classified "already present" so the new bundle record ends up with no contributed-component entries. As written, |
There was a problem hiding this comment.
🟡 Changes recommended
Local bundles cannot use the recommended update command, leaving them without an upgrade path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Prevents normal bundle installs from advancing records across bundle-version changes while components remain stale.
Changes:
- Rejects version-changing installs unless refresh mode is enabled.
- Adds regression coverage ensuring records and primitives remain unchanged.
File summaries
| File | Description |
|---|---|
src/specify_cli/bundler/services/installer.py |
Adds the version-change guard and guidance. |
tests/integration/test_bundler_install_flow.py |
Tests rejection and preserved state. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Assisted-by: OpenAI Codex (autonomous)
|
The automated review surfaced a bigger issue than my earlier note: rejecting a version-changing Please make the local-bundle case work end to end — either extend |
|
Thanks — |
There was a problem hiding this comment.
🟡 Changes recommended
The documented offline refresh route fails for non-bundled components that require catalog resolution.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
|
The |
Exercise local manifest, directory, and ZIP refresh through the real extension installer with deterministic catalog artifacts. Preserve state on offline failure and verify the online retry refreshes the owned version. Assisted-by: OpenAI Codex (model: GPT-6 Astra, autonomous)
|
Posted on behalf of @rksharma-owg by OpenAI Codex (GPT-6 Astra, autonomous). Updated in 2fd5c88: local-source refresh documentation now separates the manifest from catalog-backed component payloads. Offline errors identify the component and direct users to retry without All 115 focused tests and Ruff passed. The PR continues to use |
There was a problem hiding this comment.
🟡 Changes recommended
Same-version component-pin changes can still advance record metadata without refreshing installed payloads.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Balanced
Compare recorded component metadata with the requested plan before primitive operations. Reject changed pins, sources, preset options, and removals even when the bundle version is unchanged. Preserve idempotent installs, reordering, and additions; exercise refresh through lifecycle and real-installer CLI regressions. Assisted-by: OpenAI Codex (autonomous)
|
Posted on behalf of @rksharma-owg by OpenAI Codex (autonomous). Updated in 1d18074. Normal installs now compare the recorded owned components with the requested plan before any primitive operation, rejecting metadata changes and removals even when the bundle version is unchanged. Unchanged entries, reordering, and additions remain supported; Seventeen regression cases failed on the previous head and pass now. All 343 bundle tests pass, including real extension-installer coverage for manifest, directory, and ZIP sources with an unchanged bundle version. Ruff and the documentation lint check also pass. The full repository suite was not repeated; its earlier local limitations remain documented in the PR body. The existing review request to @mnriem is still pending. Fresh CI runs report |
|
Thanks @rksharma-owg — the same-version component-pin guard ( |
There was a problem hiding this comment.
🔵 Needs a closer look
Bundle lifecycle mutations span multiple component systems, and the updated head still requires fresh cross-platform CI validation.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Description
Refs #4434.
A normal
bundle installcan rewrite an existing bundle record while skipping its installed components. This can happen when the bundle version changes, or when a mutable manifest keeps the same bundle version but changes component pins, sources, preset options, or removes components. Removed components can be left installed with no ownership record.Reject those changes before primitive operations unless refresh is explicitly requested. Compare the existing owned
ComponentRefvalues with the requested plan, preserving idempotent installs, reordered components, and additions that leave existing owned components unchanged.Local bundles have an explicit refresh route:
specify bundle install <path> --refresh, accepting a manifest file, directory, or ZIP. This uses the existing refresh implementation to reapply owned components and remove omitted ones before updating the record, and reports refreshed and removed counts. Catalog bundles can usespecify bundle update <id>.Local sources supply manifests, not component payloads. Catalog-backed components still need network access; offline failures identify the component and instruct users to retry without
--offline.Independently installed components remain untouched and unowned under the existing no-collateral-removal policy. That second case in #4434 remains open for a separate follow-up. Refresh also retains its existing bounded rollback contract: already-installed components modified before a failure are not rolled back.
Testing
2fd5c88c: changes/removals across component kinds and same-version CLI flows for manifest, directory, and ZIP sources. They pass with the update.uvx ruff@0.15.0 check src tests: passed.npx --yes markdownlint-cli2 docs/reference/bundles.md: passed.git diff --check: passed.AI Disclosure
OpenAI Codex worked autonomously on behalf of @rksharma-owg to investigate the issue and review feedback, implement the changes and regression tests, and run validation. The changes have not received human line-by-line review from the contributor.